-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Eigen-based PnP Cost Function and Unit Test #54
Conversation
@schornakj FYI d5382d4 was @Levi-Armstrong 's fix for printing CTest output. Hopefully this will help us diagnose unexpected unit test failures more easily |
d5382d4
to
70e7f5c
Compare
rct_optimizations/include/rct_optimizations/ceres_math_utilities.h
Outdated
Show resolved
Hide resolved
70e7f5c
to
47ad8d6
Compare
Eigen::Map<const Eigen::Quaternion<T>> q(target_q); | ||
Eigen::Map<const Vector3> t(target_t); | ||
Isometry3 camera_to_target = Isometry3::Identity(); | ||
camera_to_target = Eigen::Translation<T, 3>(t) * q; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to map target_t
directly to an Eigen::Translation
instead of needing to go through an Eigen::Matrix<T,3,1>
as an intermediary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good thought. I'm not sure if the pointer maps the same way, but I can test it out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There appears to be a bug with the way that Eigen maps translations from pointers into objects, so it doesn't compile. I think we'll have to leave it as a vector for now. Ultimately I would like to be able to pass the entire matrix to a local parameterization instead of two separate objects, but I haven't worked that out yet.
Conceptually this looks good to me! I agree that the Eigen-based approach is more user-friendly than the custom Pose6d type we were using previously.
Strictly speaking the cost function could still take the quaternion and position as a single argument, you'd just need to do some pointer arithmetic when constructing the Eigen mapping. However, I agree with you that it's better to separate them, since it frees the user of the burden of needing to keep track of which indices of a raw pointer correspond to which coefficients in the parameters. |
728b9bc
to
d17c6ee
Compare
This PR adds a unit test for the 2D PnP optimization. I used this as an attempt to understand how to use Eigen objects directly in optimizations for use in the development of future cost functions (i.e. kinematic calibration). Some changes to note:
Eigen::Map
class to construct an Eigen object directly from a template data pointerOverall writing this cost function using Eigen objects was not difficult, and it makes the functions much more understandable. Also porting this change to the other cost functions would eventually allow us to phase out the custom
Pose6d
to reduce overall code size/maintenance.